enable configuring the bInterval on hid devices#1087
enable configuring the bInterval on hid devices#1087koltenpearson wants to merge 1 commit intomicropython:masterfrom
Conversation
andrewleech
left a comment
andrewleech
left a comment
There was a problem hiding this comment.
Good find, yes this interval controls the rate of messages sent to the host which can affect the responsiveness of the hid device.
Do you think a comment on the line or brief description elsewhere would help others understand what this does? I know lots of other things here aren't particularly documented either though...
Regardless there's a ci failure noting the commit message needs a little fixup to match standard project guidance.
koltenpearson
force-pushed
the
master
branch
5 times, most recently
from
d67a936 to
c3117ec
Compare
Signed-off-by: kolten <koltenpearson@fastmail.com>
koltenpearson
force-pushed
the
master
branch
from
c3117ec to
36a3ece
Compare
|
Finally got it happy with the commit message. I added a small comment as well |
| set_report_buf=None, | ||
| protocol=_INTERFACE_PROTOCOL_NONE, | ||
| interface_str=None, | ||
| binterval=8, |
There was a problem hiding this comment.
I suggest calling this new parameter just interval. The "b" means it's a byte (ie maximum 255) and I don't think that adds any value here. Using interval matches the style of the other parameters here.
(Other bits of the lower-level code use bInterval with an uppercase I, but this here is a higher-level API and I think interval is best here.)
There was a problem hiding this comment.
I would push back a little and say interval is a very generic name. Googling binterval gives you exactly what this is in the first results. Likewise it is referred to as bInterval by linux at least in all its tooling and logs.
That seems more valuable to me than consistency with the other parameter's style.
There was a problem hiding this comment.
I would push back a little and say interval is a very generic name. Googling binterval gives you exactly what this is in the first results. Likewise it is referred to as bInterval by linux at least in all its tooling and logs.
That seems more valuable to me than consistency with the other parameter's style.
That's fair enough, and I agree that searching for "binterval" gives you a lot of information about this parameter.
But by the same reasoning, protocol that's already used here is also very generic.
Given the context (USB HID device), I feel like it's quite clear what interval means, and it's great you also documented it below, so that it is clear that it's the polling rate. Maybe in the documentation it could refer to it being used as the bInterval parameter?
|
Hmm, by that same reasoning I would probably call 'protocol' a poor name ; )
Even if we are in the context of the USB HID Device class, if someone knows
enough about HID devices to be familiar with the bInterval, then calling it
binterval eliminates all ambiguity. And if they are not familiar with it,
the more specific technical name will signal to them not to assume things
they don't know without looking it up first.
While adding the bInterval designation in the comment will at least give
someone digging deeper the hint they need, making it the parameter name
adds extra benefit as it is self documenting for coding calling into it via
python keyword parameters.
And with that I have said my peace, if we aren't going to be able to agree
then I will go ahead and make the change as you suggest when I have a
moment in the next few days.
Would you prefer 'requested_polling_interval' to 'interval'? Or
'requested_polling_interval_ms' to also include the units? I still think
'binterval' is better to stay consistent with the wider ecosystem, but at
least these would be more self documenting than just 'interval', and
probably what I would choose without an already accepted name existing.
…On Thu, Mar 19, 2026 at 7:37 AM Damien George ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In micropython/usb/usb-device-hid/usb/device/hid.py
<#1087 (comment)>
:
> @@ -64,6 +64,7 @@ def __init__(
set_report_buf=None,
protocol=_INTERFACE_PROTOCOL_NONE,
interface_str=None,
+ binterval=8,
I would push back a little and say interval is a very generic name.
Googling binterval gives you exactly what this is in the first results.
Likewise it is referred to as bInterval by linux at least in all its
tooling and logs.
That seems more valuable to me than consistency with the other parameter's
style.
That's fair enough, and I agree that searching for "binterval" gives you a
lot of information about this parameter.
But by the same reasoning, protocol that's already used here is also very
generic.
Given the context (USB HID device), I feel like it's quite clear what
interval means, and it's great you also documented it below, so that it
is clear that it's the polling rate. Maybe in the documentation it could
refer to it being used as the bInterval parameter?
—
Reply to this email directly, view it on GitHub
<#1087 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADT7WT53Z2YWMFXXHV22UFL4RPZY3AVCNFSM6AAAAACV4BVWQ2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZTSNZVGE4DENJSGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
3 participants
In the process of creating a usb keyboard using the pi pico, I was not able to match the amount of HID reports that the qmk firmware could send, and eventually tracked it down to the bInterval of the qmk firmware being 1, while this one defaults to 8.
So yeah, this will let us configure it, rather than having it hardcoded